Allow CPLB to work with externalAddress #4452
Conversation
juanluisvaladas
added
the
backport/release-1.30
juanluisvaladas
force-pushed
the
cplb-extaddr
branch
4 times, most recently
from
292e70c to
473584e
Compare
Allow this behavior if the endpoint-reconciler is disabled. Signed-off-by: Juan-Luis de Sousa-Valadas Castaño <[email protected]>
juanluisvaladas
force-pushed
the
cplb-extaddr
branch
from
473584e to
9fb5dcc
Compare
Can you elaborate? K0smotron can simply use the CPLB addresses without k0s using them. Is that backport really required? |
Yes, but k0smotron would have to be modified as it's hardcoded to set the externalAddress. That's an option too.
From a purely technical point of view no, but this was requested by sales and it's important for one potential customer. I don't see many downsides to backporting so I'd say we should backport it. |
|
I think i slowly get what this is about 😅 Can you maybe write up an example use case to better explain what this PR actually unblocks? |
| @@ -92,6 +93,9 @@ func (k *Keepalived) Start(_ context.Context) error { | |||
| } | |||
|
|
|||
| if len(k.Config.VirtualServers) > 0 { | |||
| if k.HasEndpointReconciler { | |||
There was a problem hiding this comment.
What about moving that check out the component into the controller command? Otherwise the HasEndpointReconciler flag is just some "I will fail to start" flag.
| @@ -33,6 +33,8 @@ type keepalivedSuite struct { | |||
|
|
|||
| const haControllerConfig = ` | |||
| spec: | |||
| api: | |||
There was a problem hiding this comment.
Can we test both setups here? One with externalAddress and one with NLLB?
| @@ -228,24 +228,26 @@ func (c *command) start(ctx context.Context) error { | |||
| nodeComponents.Add(ctx, controllerLeaseCounter) | |||
| } | |||
|
|
|||
| disableEndpointReconciler := !slices.Contains(c.DisableComponents, constant.APIEndpointReconcilerComponentName) && | |||
There was a problem hiding this comment.
This condition is also used at line 436/438 for the endpoint reconciler component itself. Would be cool to reuse this boolean there as well. And it could deserve a less ambiguous name: Basically it swaps endpoint reconcilers. What about naming this preferExternalAddressEndpointReconciler, preferK0sEndpointReconciler or sth. like that?
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
The PR is marked as stale since no activity has been recorded in 30 days |
|
Closing this for now. Will reopen after merging userspace reverse proxy. |
Description
Allow this behavior if the endpoint-reconciler is disabled.
K0smotron needs to use external address for cluster API. The only limitation why we couldn't combine externalAddress and CPLB is the endpoint-reconciler, since it can be disabled, allow to run CPLB with externalAddress if the user manually disables it.
Type of change
How Has This Been Tested?
Checklist: